Skip to content

Conversation

@drexin
Copy link
Contributor

@drexin drexin commented Mar 30, 2021

This is a temporary workaround for segfaults we observed in TargetMetadata,
caused by invalid pointers.

This is a temporary workaround for segfaults we observed in TargetMetadata,
caused by invalid pointers.
@drexin drexin requested a review from nate-chandler March 30, 2021 20:39
@drexin
Copy link
Contributor Author

drexin commented Mar 30, 2021

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 30, 2021

cc: @buttaface

Copy link
Contributor

@nate-chandler nate-chandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@drexin
Copy link
Contributor Author

drexin commented Mar 30, 2021

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented Mar 31, 2021

@swift-ci smoke test windows

@finagolfin
Copy link
Member

I will apply this patch to my Android armv7 build of Swift 5.4 and see if it fixes it. The problem is definitely in _cacheCanonicalSpecializedMetadata, as if I comment out the two calls to that method, building Swift executables starts working again on Android armv7. Someone is going to have to step through the generated assembly for that function and see if some tweaks to its source will get the codegen optimizations working properly again, as the method works fine when unoptimized by LLVM.

@nate-chandler
Copy link
Contributor

@swift-ci smoke test

@nate-chandler
Copy link
Contributor

@swift-ci smoke test linux

@nate-chandler
Copy link
Contributor

@swift-ci smoke test linux platform

@nate-chandler nate-chandler merged commit 2f56f29 into swiftlang:main Mar 31, 2021
@drexin drexin deleted the wip-linux-arm32 branch March 31, 2021 19:20
@finagolfin
Copy link
Member

I applied this pull to the Mar. 10 Swift 5.4 snapshot for Android armv7 and hello world is still erroring out: @drexin, can you confirm this actually fixes the problem for you on linux armv7?

With the Swift stdlib built in release mode, I get this backtrace for hello world:

* thread #1, name = 'hello_toplevel', stop reason = signal SIGSEGV: invalid address (fault address: 0x1)
  * frame #0: 0xf742afd8 libswiftCore.so`swift::TargetMetadata<swift::InProcess>::getGenericArgs() const + 76
    frame #1: 0xf744535c libswiftCore.so`cacheCanonicalSpecializedMetadata(swift::TargetTypeContextDescriptor<swift::InProcess> const*, std::__ndk1::once_flag*)::$_21::__invoke(void*) + 468
    frame #2: 0xf6ce36ac libc++_shared.so`std::__ndk1::__call_once(unsigned long volatile&, void*, void (*)(void*)) + 76
    frame #3: 0xf74617d0 libswiftCore.so`swift_once + 64
    frame #4: 0xf743967c libswiftCore.so`swift_getCanonicalPrespecializedGenericMetadata + 36
    frame #5: 0xf741ee5c libswiftCore.so`__swift_instantiateCanonicalPrespecializedGenericMetadata + 48
    frame #6: 0xf701c854 libswiftCore.so`$ss23_ContiguousArrayStorageCMa + 44
    frame #7: 0xf6eba07c libswiftCore.so`$ss27_allocateUninitializedArrayySayxG_BptBwlF + 140
    frame #8: 0xaaaaa99c hello_toplevel`main + 48
    frame #9: 0xf4e101ae libc.so`__libc_init + 50
    frame #10: 0xaaaaa738 hello_toplevel`_start_main + 68

I ran the Swift compiler in lldb and checked the result of this modified shouldPrespecializeGenericMetadata() method and it always returns false now, so I don't think this change is enough.

@nate-chandler
Copy link
Contributor

That __swift_instantiateCanonicalPrespecializedGenericMetadata is being called from ss23_ContiguousArrayStorageCMa indicates that the metadata for ContiguousArray is still being prespecialized.

@finagolfin
Copy link
Member

Oh, my mistake, I'm cross-compiling the Swift stdlib on a linux x86_64 host with a prebuilt official dev snapshot compiler that doesn't have this patch, then running that stlib on Android armv7. I wasn't thinking through how the stdlib was actually compiled without this patch, never mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants